Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/65 add zero-address checks #6

Merged
merged 3 commits into from
Jul 12, 2021
Merged

Fix/65 add zero-address checks #6

merged 3 commits into from
Jul 12, 2021

Conversation

kamescg
Copy link
Contributor

@kamescg kamescg commented Jun 30, 2021

@PierrickGT
Copy link
Contributor

The following issue is also related: code-423n4/2021-06-pooltogether-findings#81

@PierrickGT PierrickGT self-assigned this Jul 1, 2021
Comment on lines 20 to 21
require(address(badgerSettAddr) != address(0), "BadgerYieldSource/badgerSettAddr-not-zero");
require(address(badgerAddr) != address(0), "BadgerYieldSource/badgerAddr-not-zero");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the isContract function from Open Zeppelin to check the addresses: https://docs.openzeppelin.com/contracts/2.x/api/utils#Address-isContract-address-

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of gas $$$ for that. I don't think it's necessary.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 994697233

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-3.6%) to 88.095%

Totals Coverage Status
Change from base Build 994692896: -3.6%
Covered Lines: 30
Relevant Lines: 30

💛 - Coveralls

@PierrickGT PierrickGT changed the base branch from master to fixes/c4-audit July 5, 2021 14:33
@PierrickGT PierrickGT merged commit e1140cc into fixes/c4-audit Jul 12, 2021
@PierrickGT PierrickGT deleted the fix/65 branch July 12, 2021 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants